Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev branch #354

Draft
wants to merge 15 commits into
base: dev-branch
Choose a base branch
from
Draft

Dev branch #354

wants to merge 15 commits into from

Conversation

Ty9000
Copy link

@Ty9000 Ty9000 commented Dec 17, 2019

No description provided.

@Ty9000
Copy link
Author

Ty9000 commented Dec 17, 2019

Just my notes:

functions.sh

  • Changed the call to buildipxe.sh to include CA cert if supplied
  • Added firewall rules instead of disabling firewall, includes DHCP check
  • Changed fog.conf to change CA file if supplied
  • Changed fog.conf to include IPXE HTTPS exception (since I can't get it to work for now)
  • Switched ServerName and ServerAlias in fog.conf
  • Added SSLVerifyClient, SSLVerifyDepth, & SSLOptions to fog.conf
    • SSLVerifyClient set to optional - will look for certificate, but does not require
  • Added update-ca-trust before Apache is restarted
  • Added ServerName directive to httpd.conf - prevents Apache throwing and error stating it can't validate the server's name
  • Added copy index.php to /var/www/html (see below)
  • Modified createSSLCA()
    • if server certificate etc is provided, copy to proper locations
  • If server certificate is provided, do check certificate for all wget calls

** Changed almost every instance of $ipaddress to $hostname (where applicable)
** Modified mysql - the $options variable didn't work for some reason
** I just manually expanded the options and passed them through for each line

index.php

  • Changed the header to /fog/management/index.php
    • Was ./management/index.php
  • This allows a user to go to the root FOG url and be redirected properly
    • Replaces the default Apache root splashscreen

installfog.sh

  • Added -v, -k, -t arguments
  • Checks to make sure all three are passed at the same time
  • Changed checkFirewall with custom function rulesFirewall (see functions.sh)

ldap.class.php

  • Checks to see if certificate used is successful
    • If not, rebind with entered username and password (local and LDAP)
    • If yes, don't rebind and proceed on

processlogin.class.php

  • Removed the 'required' element from 'username' and 'password'

@@ -19,5 +19,5 @@
* @license http://opensource.org/licenses/gpl-3.0 GPLv3
* @link https://fogproject.org
*/
header('Location: ./management/index.php');
header('Location: /fog/management/index.php');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to change this to go through /fog/ that will cause redirects to do:

http://ipoffogserver/fog/fog/fog/fog/fog/fog/management/index.php

You should be able to maintain it with ./management/index.php unless there's a specific reason for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be valid maintaining with ./management/index.php as by the time you're accessing this, you would already be in /fog/index.php so the redirect would just append ./management/index.php which would leave you in /fog/management/index.php

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, okay. Yeah, the only reason I did it was I made a change in functions.sh to copy index.php to the root of httpd so I could just go to http://fog.domain.com/ and it would redirect to /fog/management/index.php and not the Apache default splash screen. The laziest was to change the index.php in /var/www/html/fog directory and then cp it to /var/www/html/.

I just tried to go to http://fog.domain.com/fog and it redirected properly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Mind changing that back as well?

@Ty9000
Copy link
Author

Ty9000 commented Dec 17, 2019

Also, wanted to add I am working on getting OCSP configuration in order. Doing some testing and will be adding in the code tomorrow, hopefully.

@Ty9000
Copy link
Author

Ty9000 commented Dec 18, 2019

Okay, I got the OCSP configuration sorted out and uploaded.

There are a few caveats for more secure environments.

  1. SSLVerifyClient is set to optional, therefore this allows a user to not select a certificate and login with the local user(s) or with LDAP only. If a user has a certificate, I don't see why a user would ever need to do this if he/she has a certificate, especially in an environment with virtual or physical smart cards where login is only available via the smart card.
  2. The above can be remedied with setting SSLVerifyClient to require; however, local and LDAP login is prohibited. Also, the backupDB function in functions.sh no longer works and I'd imagine the rest of the function calls that use wget don't work. I cannot for the life of me figure out how to make it work with certificates. Maybe someone with more experience can figure it out.
  3. For the OCSP configuration, I was unable to figure out how to verify my intermediate CA's certificate against my current OCSP server. I know there is a way to do it, but I can't find anything when searching the Internet - I'm sure I just need to post in a forum somewhere and someone will know the answer. Because of this, I set SSLOCSPEnable to leaf, which only verifies the client's certificate and not the chain.
  4. Same for OCSP configuration, in Windows nonce is not enabled by default (but in Linux OCSP servers nonce is enabled) and I disabled the use of nonce when fog.conf is written. Nonce in Windows OCSP can be enabled extremely easily (it's just a checkbox and a reboot), but better safe than sorry, I'm sure.
  5. I'm certain that if anyone dives this deeply into making their FOG server more secure, they probably have some Linux knowledge and can make the appropriate changes to suit their environment, I hope.

Copy link
Member

@Sebastian-Roth Sebastian-Roth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Thanks again for working on this and sending in your changes! There are a couple of things we will consider adding to the official code base. Though we can't just merge it in as is because it would break thinks for some users I think.
I'd prefer to just pull in some of your changes but also drop parts of the pull request. Would you be fine with that or should we rather discuss all the things to the point where we agree to merge it into the official code base as is?

@@ -630,7 +709,7 @@ while [[ -z $blGo ]]; do
echo
echo " This can be done by opening a web browser and going to:"
echo
echo " ${httpproto}://${ipaddress}${webroot}management"
echo " ${httpproto}://${hostname}${webroot}management"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 You probably changed to hostname because you use a proper SSL certificate with hostname in it. I think we'll stick to IP address for now because we cannot rely on people having a proper DNS configuration for their server. We'd probably need to ask during setup.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! That's my bad. I'm just so used to having DNS as being authoritative, I made those changes for my own sanity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we provide both instances:
This can be done by opening a browser to either on or the other......

If you built your own certificate you can test this by using the hostname.
${httpproto}://${ipaddress}${webroot}management
${httpproto}://${hostname}${webroot}management

@@ -1054,7 +1097,7 @@ configureMySql() {
[[ -n $snmysqlpass ]] && options=( "${options[@]}" "--password=$snmysqlpass" )
sqlescsnmysqlpass=$(echo "$snmysqlpass" | sed -e s/\'/\'\'/g) # Replace every ' with '' for full MySQL escaping
sql="UPDATE mysql.user SET plugin='mysql_native_password' WHERE User='root';"
mysql "${options}" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1
mysql -s --host="$snmysqlhost" --user="$snmysqluser" --password="$snmysqlpass" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 I have done a fair amount of work on the mysql stuff. This will be merged into the code soon. I am not sure why you changed it from using the $options array to single parameters?!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the $options variable didn't build properly and I was getting errors stating the mysql code was attempting to log in as root with no password. I did add a password to mysql and I added it to .fogsettings, but $options never built up properly even though the variables that constitute $options were passing through correctly. Very strange...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 This issue is known and as mentioned I did a complete rewrite of the database stuff in branch db-security - will merge that into dev-branch in the next days. This should fix the problem altogether.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 May I ask you to revert all those mysql changes? As mentioned I have pushed a big change for this part of the code already.

mkdir -p $webdirdest/management/other/ssl >>$workingdir/error_logs/fog_error_${version}.log 2>&1
cp -f "$serverCert" $webdirdest/management/other/ssl/srvpublic.crt >>$workingdir/error_logs/fog_error_${version}.log 2>&1
cp -f "$serverKey" $sslpath/.srvprivate.key >>$workingdir/error_logs/fog_error_${version}.log 2>&1
cp -f "$externalCA" /etc/pki/ca-trust/source/anchors/chain.pem >>$workingdir/error_logs/fog_error_${version}.log 2>&1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 The path /etc/pki/ca-trust/... is only valid fog RedHat based systems. We'll definitely make this more general for Debian/Ubuntu and ARCH Linux. Why did you decide to copy your CA to the system CA folder? Wouldn't it be easier to copy to /opt/fog/... because that path is the same for all Linux systems that use FOG. But possibly there are other programs that look for the CA in the system folder?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in the /etc/pki... folder so when update-ca-trust ran (which I added later in the code), it would add the custom certificate authorities to the complete listing of CAs on the machine and the machine certificate would be trusted

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll work on getting this more generalized for Debian to start with. I'm working on generalizing the firewall rules first and then I will move on to this. Anyone, is there anything else I've added that is specifically RedHat that needs to be generalized? I didn't see anything, but want to make sure.

I'll move on to ARCH once I test Debian.

echo " SSLCertificateKeyFile $sslprivkey" >> "$etcconf"
echo " SSLCertificateChainFile $webdirdest/management/other/ca.cert.der" >> "$etcconf"
if [ ! -z "$externalCA" ]; then
echo " SSLCACertificateFile /etc/pki/ca-trust/source/anchors/chain.pem" >> "$etcconf"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 I am not exactly sure but I think the options SSLCACertificateFile and SSLCertificateChainFile are specific to Apache version. The later might even be wrong in the context we use it. See here: https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatechainfile

SSLCertificateChainFile became obsolete with version 2.4.8, when SSLCertificateFile was extended to also load intermediate CA certificates from the server certificate file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSLCertificateChainFile is the default code you guys put in, so I just reused it if the user does not input an external CA certificate chain. Is that not right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 This part of the code is likely old (from pre 2.4.x Apache days). So my guess is this was put in and not touched since then as it didn't cause any harm. Apache seems to work fine even without this option being correct.

@@ -1906,6 +1975,7 @@ configureHttpd() {
esac
fi
dots "Setting up Apache and PHP files"
echo "ServerName $hostname" | sudo tee -a /etc/httpd/conf/httpd.conf >> /dev/null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 If you run the installer more than once (either because of an error or on upgrades) it will append that same line over and over again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! I'll need to do a check on that line's existence and then react to that...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think I fixed it. I uploaded a new version of functions.sh

$this->_username = $uname;
$this->_password = $upass;
global $currentUser;
$user = $this->hasValidCert();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 I think there is a cleaner way of doing an auto login and not show the login page at all. I will let you know when I have a good solution to this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10-4, sounds good. Thanks!

Also, just making sure you can see my first post? I can't tell since I'm so new to Github... It's where I put an overview of changes and a general reasoning as to why I made the changes in some locations.

. 'required="" id="upass"/>';
//echo '<input type="password" class="form-control" name="upass" '
// . 'required="" id="upass"/>';
echo '<input type="password" class="form-control" name="upass" id="upass"/>';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about handling this with a little javascript and maybe a "login method" drop down?

For now there'd only be 2 dropdowns (username/password, certificate)

If certificate is selected, the password field becomes hidden using the javascript, if username/password is selected the field becomes visible/required.

Just thoughts. Using the drop down method would also allow us to add other authentication mechanisms in the future as well.

@@ -750,6 +774,25 @@ checkSELinux() {
esac
done
}
rulesFirewall() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is very Redhat centric right now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Haha I'll work on the Debian-based stuff soon. I don't have much experience with Debian, but I'll give it a shot.

@@ -1906,6 +1975,9 @@ configureHttpd() {
esac
fi
dots "Setting up Apache and PHP files"
if (! grep -Fxq "ServerName \"$hostname\"" /etc/httpd/conf/httpd.conf); then
echo "ServerName \"$hostname\"" | sudo tee -a /etc/httpd/conf/httpd.conf >> /dev/null
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I have a variable to get the httpd conf, which is ${httpdconf}

This said, you're forcing the Servername to the main httpd.conf, why not do this for the fog configuration file we generate?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I was seeing this error in Apache's log for a while and then it just stopped over a week ago. I think it's because I flipped the ServerName and ServerAlias when fog.conf is written. The original has the IP address as the ServerName and the hostname as the ServerAlias. I removed this line and reuploaded functions.sh

@Ty9000
Copy link
Author

Ty9000 commented Jan 16, 2020

Scrapped the firewall rules. I can't figure out how to properly use iptables and it seems like it's very difficult to get it to work properly with NFS. Firewall-cmd doesn't work on the latest version of iptables on Debian, so I just said forget it for now.

I did add a few checks for Debian and tried to tailor the functions to that OS. It seems Arch and Red Hat share common functions for everything used by FOG, so I lumped them together. Not sure if that is entirely accurate, so please let me know if I messed anything up.

Removed the check for server certificate and the wget calls since I couldn't get that to work properly either right now.

@@ -36,7 +36,7 @@ registerStorageNode() {
if [[ $storageNodeExists != exists ]]; then
[[ -z $maxClients ]] && maxClients=10
dots "Node being registered"
wget --no-check-certificate -qO - $httpproto://$ipaddress/${webroot}/maintenance/create_update_node.php --post-data="newNode&name=$(echo -n $ipaddress| base64)&path=$(echo -n $storageLocation|base64)&ftppath=$(echo -n $storageLocation|base64)&snapinpath=$(echo -n $snapindir|base64)&sslpath=$(echo -n $sslpath|base64)&ip=$(echo -n $ipaddress|base64)&maxClients=$(echo -n $maxClients|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&interface=$(echo -n $interface|base64)&bandwidth=$(echo -n $interface|base64)&webroot=$(echo -n $webroot|base64)&fogverified"
wget --no-check-certificate -qO - $httpproto://$ipaddress/${webroot}/maintenance/create_update_node.php --post-data="newNode&name=$(echo -n $ipaddress| base64)&path=$(echo -n $storageLocation|base64)&ftppath=$(echo -n $storageLocation|base64)&snapinpath=$(echo -n $snapindir|base64)&sslpath=$(echo -n $sslpath|base64)&ip=$(echo -n $ipaddress|base64)&maxClients=$(echo -n $maxClients|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&interface=$(echo -n $interface|base64)&bandwidth=$(echo -n $interface|base64)&webroot=$(echo -n $webroot|base64)&fogverified"
echo "Done"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Please correct indent here and on the following wget call lines (spaces at the beginning of the line).

@@ -46,13 +46,14 @@ updateStorageNodeCredentials() {
[[ -z $webroot ]] && webroot="/"
dots "Ensuring node username and passwords match"
wget --no-check-certificate -qO - $httpproto://$ipaddress${webroot}maintenance/create_update_node.php --post-data="nodePass&ip=$(echo -n $ipaddress|base64)&user=$(echo -n $username|base64)&pass=$(echo -n $password|base64)&fogverified"
fi
echo "Done"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 This if statement might cause a script error I reckon.

@@ -417,7 +418,7 @@ configureFTP() {
}
configureDefaultiPXEfile() {
[[ -z $webroot ]] && webroot='/'
echo -e "#!ipxe\ncpuid --ext 29 && set arch x86_64 || set arch \${buildarch}\nparams\nparam mac0 \${net0/mac}\nparam arch \${arch}\nparam platform \${platform}\nparam product \${product}\nparam manufacturer \${product}\nparam ipxever \${version}\nparam filename \${filename}\nparam sysuuid \${uuid}\nisset \${net1/mac} && param mac1 \${net1/mac} || goto bootme\nisset \${net2/mac} && param mac2 \${net2/mac} || goto bootme\n:bootme\nchain ${httpproto}://$ipaddress${webroot}service/ipxe/boot.php##params" > "$tftpdirdst/default.ipxe"
echo -e "#!ipxe\ncpuid --ext 29 && set arch x86_64 || set arch \${buildarch}\nparams\nparam mac0 \${net0/mac}\nparam arch \${arch}\nparam platform \${platform}\nparam product \${product}\nparam manufacturer \${product}\nparam ipxever \${version}\nparam filename \${filename}\nparam sysuuid \${uuid}\nisset \${net1/mac} && param mac1 \${net1/mac} || goto bootme\nisset \${net2/mac} && param mac2 \${net2/mac} || goto bootme\n:bootme\nchain http://${hostname}/fog/service/ipxe/boot.php##params" > "$tftpdirdst/default.ipxe"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Please switch back to use ${httpproto} variable.

@@ -1054,7 +1097,7 @@ configureMySql() {
[[ -n $snmysqlpass ]] && options=( "${options[@]}" "--password=$snmysqlpass" )
sqlescsnmysqlpass=$(echo "$snmysqlpass" | sed -e s/\'/\'\'/g) # Replace every ' with '' for full MySQL escaping
sql="UPDATE mysql.user SET plugin='mysql_native_password' WHERE User='root';"
mysql "${options}" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1
mysql -s --host="$snmysqlhost" --user="$snmysqluser" --password="$snmysqlpass" -e "$sql" >>$workingdir/error_logs/fog_error_${version}.log 2>&1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 May I ask you to revert all those mysql changes? As mentioned I have pushed a big change for this part of the code already.

@@ -1728,19 +1749,37 @@ EOF
echo " SetHandler \"proxy:fcgi://127.0.0.1:9000/\"" >> "$etcconf"
fi
echo " </FilesMatch>" >> "$etcconf"
echo " ServerName $ipaddress" >> "$etcconf"
echo " ServerAlias $hostname" >> "$etcconf"
echo " ServerName $hostname" >> "$etcconf"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Please switch back name and alias as most FOG users still use IP address.

cp -f $webdirsrc/index.php /var/www/
else
cp -f $webdirsrc/index.php /var/www/html/
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Why was this added?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think when I tested with Debian, Apache's root directory is different from Red Hat's.

@@ -2094,7 +2143,7 @@ class Config
*/
private static function _initSetting()
{
define('TFTP_HOST', \"${ipaddress}\");
define('TFTP_HOST', \"${hostname}\");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Please change back to IP for the official code. A lot of our users have a hard time setting FOG up and I think it's too much for a lot of them to get DNS right on top of that.

@@ -19,5 +19,5 @@
* @license http://opensource.org/licenses/gpl-3.0 GPLv3
* @link https://fogproject.org
*/
header('Location: ./management/index.php');
header('Location: /fog/management/index.php');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ty9000 Mind changing that back as well?

@Sebastian-Roth
Copy link
Member

@Ty9000 Are you still active and want to get this merged into the code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants